Skip to content

Conversation

@brendandahl
Copy link
Collaborator

Implement a caching mechanism using a JSON file to track the state of downloaded models. This avoids redundant downloads during prebuild if the model configuration has not changed.

A --force flag is also added to allow users to manually bypass the cache and re-download all models when necessary.

Implement a caching mechanism using a JSON file to track the state of
downloaded models. This avoids redundant downloads during prebuild if
the model configuration has not changed.

A --force flag is also added to allow users to manually bypass the cache
and re-download all models when necessary.
@brendandahl brendandahl requested a review from rmahdav January 22, 2026 21:54
fs.mkdirSync(MODEL_DIR, { recursive: true });
}

if (!forceDownload && fs.existsSync(CACHE_FILE)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR is a great improvement to the benchmark, thanks Brendan!

However, this looks like all-or-nothing situation now. Meaning, if just one of the models has changed, all the models will be downloaded again. I think it is better if we check the model changes and skip downloading them one by one. Something like:

for (const modelInfo of MODELS_TO_DOWNLOAD) {
...
        const cachedEntry = cachedModels.find(m => m.filename === filename && m.repo === repo);
        const configMatches = cachedEntry && JSON.stringify(cachedEntry) === JSON.stringify(modelInfo);
        const fileExists = fs.existsSync(outputPath);

        if (!forceDownload && configMatches && fileExists) {
            console.log(`Model **${filename}** is up to date. Skipping.`);
            newCache.push(modelInfo);
            continue;
        }
// Download if missing or changed
...        
}

(The code snippet is Gemini suggested to my change request, so take it with a grain of salt)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, I was trying to do something quick for testing. It's pretty easy to do per file though, so I'll change it. I added a little helper class in this other PR. Once that lands I'll update this one do the same thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants